fix(bridge-react): prevent react-dom/client being bundled #3476

francisduvivier · 2025-02-03T22:15:46Z

This otherwise causes the package to be incompatible with a react 19

BREAKING CHANGE: react-dom/client code will not be bundled into the bridge-react module anymore

Description

This PR changes the vite rollup config for the bridge-react package in order not to bundle react-dom/client.
I assume that this inclusion into the bundle was accidental, since react-dom/client is part of react-dom which is a peer dependency, and peer dependencies are marked as external already in the rollup config.

The inclusion of this v18 react-dom/client module code also hinders compatibility with remote react v19 modules that want to use react bridge. So this PR fixes that part of imcompatibility with react v19, I don't know if there is other issues with react v19 though.

To see what I mean, you can look at the bridge-react/dist/index.es.js

Technically this fix can be breaking if someone somehow depends on the react-dom/client code being bundled into the @module-federation/bridge-react package.

Related Issue

#3477

Types of changes

Docs change / refactoring / dependency upgrade
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Checklist

I have added tests to cover my changes.
All new and existing tests passed.
I have updated the documentation.

changeset-bot · 2025-02-03T22:15:51Z

🦋 Changeset detected

Latest commit: dc99ab0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 29 packages

Name	Type
@module-federation/bridge-react	Patch
@module-federation/runtime	Patch
@module-federation/enhanced	Patch
@module-federation/rspack	Patch
@module-federation/webpack-bundler-runtime	Patch
@module-federation/sdk	Patch
@module-federation/runtime-tools	Patch
@module-federation/managers	Patch
@module-federation/manifest	Patch
@module-federation/dts-plugin	Patch
@module-federation/third-party-dts-extractor	Patch
@module-federation/devtools	Patch
@module-federation/bridge-vue3	Patch
@module-federation/bridge-shared	Patch
@module-federation/bridge-react-webpack-plugin	Patch
@module-federation/modern-js	Patch
@module-federation/retry-plugin	Patch
@module-federation/data-prefetch	Patch
@module-federation/rsbuild-plugin	Patch
@module-federation/error-codes	Patch
@module-federation/inject-external-runtime-core-plugin	Patch
@module-federation/nextjs-mf	Patch
@module-federation/node	Patch
@module-federation/storybook-addon	Patch
@module-federation/modernjsapp	Patch
@module-federation/esbuild	Patch
@module-federation/runtime-core	Patch
@module-federation/utilities	Patch
website-new	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

netlify · 2025-02-03T22:17:26Z

✅ Deploy Preview for module-federation-docs ready!

Name	Link
🔨 Latest commit	`dc99ab0`
🔍 Latest deploy log	https://app.netlify.com/sites/module-federation-docs/deploys/67a4ac7d738381000732e9a7
😎 Deploy Preview	https://deploy-preview-3476--module-federation-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

francisduvivier · 2025-02-04T11:45:41Z

I skipped some of the steps like adding tests since that seems a bit much for this, but let me know if some extra work is needed from my side for this to be merged.

danpeen · 2025-02-05T08:32:24Z

@francisduvivier hello, thanks for your pr. And I have checked @module-federation/bridge-react packages bundle config, i think in the rollupOptions.external configs we have already set the peerDependencies as external packages, which of course include react-dom and should not be bundled into the final assets. So i think react-dom/client will not be bundled either. So i was wondering how you find that react-dom/client packages being bundled and if it is i think that is not expectable, please give me an example so i can offer some help.

francisduvivier · 2025-02-05T15:46:06Z

Hi @danpeen, that is a good question, I have tried my best to specify it in the reproduction part in the related issue:
#3477

Reproduction

Check out this repository

build the bridge-react package

check the imports in bridge-react/dist/index.es.js

You will see that there is no import for react-dom/client, instead the code is bundled below containing __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED on line 333

Alternatively to the steps above you can also check the bundle at https://www.npmjs.com/package/@module-federation/bridge-react?activeTab=code and then @module-federation/bridge-react/dist/index.es.js line 333

danpeen · 2025-02-06T04:48:17Z

Hi @francisduvivier, i got what you mean. Here is the thing:

this piece of code wraps the entry point of react-dom/client, mainly wrapping two methods: createRoot and hydrateRoot.
it is a wrapper layer, rather than directly packaging react-dom/client.
the key point is the m variable, which will be imported from the outside as a react-dom, not packaged in.

and you can try even if we set the react-dom/* as the external packages as the pr changes, you will see it will act the same as the old config.

and if we truly bundled the react or react-dom into the final assets, i think you can find code like this
function createRoot(container, options) { ... } which contains a large number of specific API's implementation

francisduvivier · 2025-02-06T08:11:28Z

Hi @danpeen, so some points:

As I see it, the code you see does not "wrap" the entry point of react-dom/client, it "is" the entry point code of react-dom/client and it wraps react-dom: I think it comes from here: https://github.com/facebook/react/blob/v18.3.1/packages/react-dom/client.js
I believe the part that you are missing and which I also found quite a gotcha is that the rollup external settings work in a quircky way where direct imports from the package are prevented from bundling, but imports with a / after that package name are not, and in the case of react-dom/client, that gives the unwanted behaviour of including some react-dom code unintentionally unless you also specify it explicitly as external.
You can check out the code, run the build and then check the new dist/index.es.js file to verify that that part in your screenshot is not bundled in anymore.

Kind regards,
Francis

sodehl · 2025-02-06T08:54:23Z

Hi @francisduvivier, thanks for your solution. We ran into the same problem yesterday and your solution worked for us.

@danpeen Because the bridge imports react-dom/client (provider.tsx line 4), the exact import statement or a regex that matches it must be configured. So, putting the peerDependencies (react-dom) in the external array is not enough in this case.

The rollup documentation for 'external' states: "the name of an external dependency, exactly the way it is written in the import statement."

danpeen · 2025-02-06T12:33:55Z

@francisduvivier @sodehl Thank you very much!!!

Now i totally understand what is going on, yes we really need to add react-dom/client to the external list and then rollup will exclude this package, and i've also verified this.

now let's make this pr in!

Thankyou again!!

Kind regards,
danping.

…ndex.*.js this otherwise causes the package to be incompatible with a react 19 BREAKING CHANGE: react-dom/client code will not be bundled into the bridge-react module anymore

francisduvivier mentioned this pull request Feb 3, 2025

@module-federation/bridge-react package bundles react-dom/client (v18) code #3477

Closed

5 tasks

francisduvivier force-pushed the fix/bridge-prevent-react-dom-client-bundling branch from 313644e to f29b319 Compare February 4, 2025 07:16

fix(bridge-react): prevent react-dom/client being bundled into dist/i…

dc99ab0

…ndex.*.js this otherwise causes the package to be incompatible with a react 19 BREAKING CHANGE: react-dom/client code will not be bundled into the bridge-react module anymore

danpeen force-pushed the fix/bridge-prevent-react-dom-client-bundling branch from f29b319 to dc99ab0 Compare February 6, 2025 12:35

danpeen approved these changes Feb 6, 2025

View reviewed changes

danpeen merged commit e751ad0 into module-federation:main Feb 6, 2025
14 checks passed

KyrieLii mentioned this pull request Feb 7, 2025

chore: release 0.8.10 #3491

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(bridge-react): prevent react-dom/client being bundled #3476

fix(bridge-react): prevent react-dom/client being bundled #3476

francisduvivier commented Feb 3, 2025 •

edited

Loading

changeset-bot bot commented Feb 3, 2025 •

edited

Loading

netlify bot commented Feb 3, 2025 •

edited

Loading

francisduvivier commented Feb 4, 2025

danpeen commented Feb 5, 2025

francisduvivier commented Feb 5, 2025 •

edited

Loading

danpeen commented Feb 6, 2025 •

edited

Loading

francisduvivier commented Feb 6, 2025 •

edited

Loading

sodehl commented Feb 6, 2025

danpeen commented Feb 6, 2025

fix(bridge-react): prevent react-dom/client being bundled #3476

fix(bridge-react): prevent react-dom/client being bundled #3476

Conversation

francisduvivier commented Feb 3, 2025 • edited Loading

Description

Related Issue

Types of changes

Checklist

changeset-bot bot commented Feb 3, 2025 • edited Loading

🦋 Changeset detected

netlify bot commented Feb 3, 2025 • edited Loading

✅ Deploy Preview for module-federation-docs ready!

francisduvivier commented Feb 4, 2025

danpeen commented Feb 5, 2025

francisduvivier commented Feb 5, 2025 • edited Loading

danpeen commented Feb 6, 2025 • edited Loading

francisduvivier commented Feb 6, 2025 • edited Loading

sodehl commented Feb 6, 2025

danpeen commented Feb 6, 2025

francisduvivier commented Feb 3, 2025 •

edited

Loading

changeset-bot bot commented Feb 3, 2025 •

edited

Loading

netlify bot commented Feb 3, 2025 •

edited

Loading

francisduvivier commented Feb 5, 2025 •

edited

Loading

danpeen commented Feb 6, 2025 •

edited

Loading

francisduvivier commented Feb 6, 2025 •

edited

Loading